-
-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Execute a file instead of code fragments #25
base: master
Are you sure you want to change the base?
Conversation
@@ -49,3 +50,34 @@ def inject_py(pid: int, python_code: AnyStr, permissions=0o644) -> None: | |||
finally: | |||
if path is not None and path.exists(): | |||
path.unlink() | |||
|
|||
def inject_py_script(pid: int, python_script: Path, permissions=0o644) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much of the logic here is duplicated from above. I think it might be better to keep it as one function, inject_py
, and check isinstance(python_code, Path)
to determine the logic. Also, I think that instead of modifying injection.c
, it would be easier to just read the given file here, and use the contents as code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a large script is provided then it will overflow. Also relative imports will not work as expected either presumably. To me it made sense to use the Python API file parsing and execution instead of shoehorning the code into the lib?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree about the advantages, but ideally I'd want to enocunter all possible errors before the injection, so they happen in python and not in C in the target process. I also wonder permission-wise, if it's better to use the injector process permissions or the injectee's permissions (and fs, btw, see #18). I will hopefully have more time to think about it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree about the advantages, but ideally I'd want to enocunter all possible errors before the injection, so they happen in python and not in C in the target process.
Are there more errors this way? I am not sure I understand where the source of errors you're concerned about is.
As for Docker, I am already running into some issues running from within the container and being able to open libc.so
. Containerisation always makes things complicated 😅
Thanks for the feature! I fixed CI on master, you can update your branch |
Not sure what is going on with the alpine, Mac and Windows tests. Something is causing the process to segfault in some cases, groan. |
I'm flying abroad, so sadly I can't help right now, would love to help merge this whenever I can |
You can now inject a python script path. This seemed easier to manage than trying to get my scripts as arguments on the command line.
Platforms other than Linux are not tested.